Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

recur_expansion.js: fix iterator with RDATE-only recurrence #534

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

recur_expansion.js: fix iterator with RDATE-only recurrence #534

wants to merge 1 commit into from

Conversation

dilyanpalauzov
Copy link
Contributor

When iterating with

  e is an event-component
  if (e.isRecurring() {
     let i = e.iterator();
     while (n = i.next()) {
       o = g.getOccurrenceDetails(obj)
       …
     }
  }

on the first iteration the DTSTART instance shall be returned. It is returned, when there is RRULE. In the lack of this change, on the first iteration, when RDATE is present without RRULE, the first RDATE instance is returned.

@darktrojan how does Thunderbird handle VEVENTS with RDATE but without RRULE. These are presented correct in the UI. But the instances cannot be obtained by getting an iteratior of the event and then calling .next(). The reason is, that the very first iteration skips then the DTSTART-instance. The DTSTART instance is not skipped by the iterator.next() when the VEVENT has RRULE.

dilyanpalauzov added a commit to dilyanpalauzov/ical.js that referenced this pull request Sep 16, 2022
@dilyanpalauzov
recur_expansion.js: fix iterator with RDATE-only recurrence
c1ee71d 
kewisch#534

When iterating with
  e is an event-component
  if (e.isRecurring() {
     let i = e.iterator();
     while (n = i.next()) {
       o = g.getOccurrenceDetails(obj)
       …
     }
  }

on the first iteration the DTSTART instance shall be returned.  It is returned,
when there is RRULE.  In the lack of this change, on the first iteration, when
RDATE is present without RRULE, the first RDATE instance is returned.
dilyanpalauzov added a commit to dilyanpalauzov/ical.js that referenced this pull request Sep 17, 2022
@dilyanpalauzov
recur_expansion.js: fix iterator with RDATE-only recurrence
9a9ff7a 
kewisch#534

When iterating with
  e is an event-component
  if (e.isRecurring() {
     let i = e.iterator();
     while (n = i.next()) {
       o = g.getOccurrenceDetails(obj)
       …
     }
  }

on the first iteration the DTSTART instance shall be returned.  It is returned,
when there is RRULE.  In the lack of this change, on the first iteration, when
RDATE is present without RRULE, the first RDATE instance is returned.
dilyanpalauzov added a commit to dilyanpalauzov/ical.js that referenced this pull request Sep 18, 2022
@dilyanpalauzov
recur_expansion.js: fix iterator with RDATE-only recurrence
a979f58 
kewisch#534

When iterating with
  e is an event-component
  if (e.isRecurring() {
     let i = e.iterator();
     while (n = i.next()) {
       o = g.getOccurrenceDetails(obj)
       …
     }
  }

on the first iteration the DTSTART instance shall be returned.  It is returned,
when there is RRULE.  In the lack of this change, on the first iteration, when
RDATE is present without RRULE, the first RDATE instance is returned.
@coveralls
Copy link

coveralls commented Sep 18, 2022 •
edited
Loading

Pull Request Test Coverage Report for Build 3076542534

  • 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 95.473%
Totals Coverage Status
Change from base Build 3012316465: 0.03%
Covered Lines: 2932
Relevant Lines: 3023
💛 - Coveralls

@dilyanpalauzov
recur_expansion.js: fix iterator with RDATE-only recurrence
4f5c2d6 
When iterating with
  e is an event-component
  if (e.isRecurring() {
     let i = e.iterator();
     while (n = i.next()) {
       o = g.getOccurrenceDetails(obj)
       …
     }
  }

on the first iteration the DTSTART instance shall be returned.  It is returned,
when there is RRULE.  In the lack of this change, on the first iteration, when
RDATE is present without RRULE, the first RDATE instance is returned.
dilyanpalauzov added a commit to dilyanpalauzov/ical.js that referenced this pull request Sep 18, 2022
@dilyanpalauzov
recur_expansion.js: fix iterator with RDATE-only recurrence
765ae1b 
kewisch#534

When iterating with
  e is an event-component
  if (e.isRecurring() {
     let i = e.iterator();
     while (n = i.next()) {
       o = g.getOccurrenceDetails(obj)
       …
     }
  }

on the first iteration the DTSTART instance shall be returned.  It is returned,
when there is RRULE.  In the lack of this change, on the first iteration, when
RDATE is present without RRULE, the first RDATE instance is returned.
@darktrojan
Copy link
Collaborator

@darktrojan how does Thunderbird handle VEVENTS with RDATE but without RRULE. These are presented correct in the UI. But the instances cannot be obtained by getting an iteratior of the event and then calling .next(). The reason is, that the very first iteration skips then the DTSTART-instance. The DTSTART instance is not skipped by the iterator.next() when the VEVENT has RRULE.

Thunderbird doesn't use this code for RDATEs, it maintains a list of them and does iteration itself. So I'm not familiar with how this bit works at all.

darktrojan
darktrojan requested changes Nov 9, 2022
View reviewed changes
Copy link
Collaborator

@darktrojan darktrojan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my very limited understanding of how this particular bit works, this looks okay to me. But it needs updating for ES6.

dilyanpalauzov added a commit to dilyanpalauzov/ical.js that referenced this pull request Nov 29, 2022
@dilyanpalauzov
recur_expansion.js: fix iterator with RDATE-only recurrence
25ec99a 
kewisch#534

When iterating with
  e is an event-component
  if (e.isRecurring() {
     let i = e.iterator();
     while (n = i.next()) {
       o = g.getOccurrenceDetails(obj)
       …
     }
  }

on the first iteration the DTSTART instance shall be returned.  It is returned,
when there is RRULE.  In the lack of this change, on the first iteration, when
RDATE is present without RRULE, the first RDATE instance is returned.
dilyanpalauzov added a commit to dilyanpalauzov/ical.js that referenced this pull request Nov 29, 2022
@dilyanpalauzov
recur_expansion.js: fix iterator with RDATE-only recurrence
93d5911 
kewisch#534

When iterating with
  e is an event-component
  if (e.isRecurring() {
     let i = e.iterator();
     while (n = i.next()) {
       o = g.getOccurrenceDetails(obj)
       …
     }
  }

on the first iteration the DTSTART instance shall be returned.  It is returned,
when there is RRULE.  In the lack of this change, on the first iteration, when
RDATE is present without RRULE, the first RDATE instance is returned.
@titanism titanism mentioned this pull request Feb 16, 2024
Closed
dilyanpalauzov added a commit to dilyanpalauzov/ical.js that referenced this pull request Apr 13, 2024
@dilyanpalauzov
recur_expansion.js: fix iterator with RDATE-only recurrence
800344c 
kewisch#534

When iterating with
  e is an event-component
  if (e.isRecurring() {
     let i = e.iterator();
     while (n = i.next()) {
       o = g.getOccurrenceDetails(obj)
       …
     }
  }

on the first iteration the DTSTART instance shall be returned.  It is returned,
when there is RRULE.  In the lack of this change, on the first iteration, when
RDATE is present without RRULE, the first RDATE instance is returned.
@kewisch
Copy link
Owner

kewisch commented Apr 13, 2024

@dilyanpalauzov looks like you are actively working on this one, anything I can do to help get this merged?

dilyanpalauzov added a commit to dilyanpalauzov/ical.js that referenced this pull request Apr 14, 2024
@dilyanpalauzov
recur_expansion.js: fix iterator with RDATE-only recurrence
307bc07 
kewisch#534

When iterating with
  e is an event-component
  if (e.isRecurring() {
     let i = e.iterator();
     while (n = i.next()) {
       o = g.getOccurrenceDetails(obj)
       …
     }
  }

on the first iteration the DTSTART instance shall be returned.  It is returned,
when there is RRULE.  In the lack of this change, on the first iteration, when
RDATE is present without RRULE, the first RDATE instance is returned.
@dilyanpalauzov
Copy link
Contributor Author

This change for proposal is one and a half years old. I do not know what is missing to get it merged. I see now it contains a test, which is failing. Sorry, I do not know anymore what was going on.

@kewisch
Copy link
Owner

kewisch commented Apr 14, 2024

Ah no problem, I had seen a recently referenced commit which made me assume. If you do feel like picking this up again I'm happy to give reviews some priority.

@dilyanpalauzov dilyanpalauzov mentioned this pull request Apr 21, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@darktrojan darktrojan darktrojan requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dilyanpalauzov @coveralls @darktrojan @kewisch